Skip to content

Ensure all relevant SaveLockedStateStore functions are synchronized with SaveLockedStateStore::save_changes#6547

Open
mgoldenberg wants to merge 3 commits intomatrix-org:mainfrom
mgoldenberg:sync-other-save-locked-state-store-fns
Open

Ensure all relevant SaveLockedStateStore functions are synchronized with SaveLockedStateStore::save_changes#6547
mgoldenberg wants to merge 3 commits intomatrix-org:mainfrom
mgoldenberg:sync-other-save-locked-state-store-fns

Conversation

@mgoldenberg
Copy link
Copy Markdown
Contributor

@mgoldenberg mgoldenberg commented May 7, 2026

Since #6478, the BaseStateStore uses a SaveLockedStateStore to ensure that calls to the underlying StateStore::save_changes are synchronized. It likely makes sense, however, to synchronize calls to other StateStore functions which may interfere with StateStore::save_changes.

Changes

  • Update SaveLockedStateStore::remove_room so that it acquires the lock before issuing a call to the underlying store.
  • Add a function, SaveLockedStateStore::remove_room_with_guard for cases where the lock has already been acquired and the caller would like to remove a room from the underlying store.

Considerations

Below is a list of other StateStore functions which write to the database. It does not seem like any of these would interfere with StateStore::save_changes, but they are listed below for examination by the reviewer, in case I have overlooked something.

  1. set_kv_data
  2. remove_kv_data
  3. set_custom_value
  4. remove_custom_value
  5. save_send_queue_request
  6. update_send_queue_request
  7. remove_send_queue_request
  8. update_send_queue_request_status
  9. save_dependent_queued_request
  10. mark_dependent_queued_requests_as_ready
  11. update_dependent_queued_request
  12. remove_dependent_queued_request
  13. upsert_thread_subscription
  14. remove_thread_subscription
  15. set_custom_value_no_read

Closes #6546.

  • I've documented the public API Changes in the appropriate CHANGELOG.md files.
  • This PR was made with the help of AI.

Signed-off-by: Michael Goldenberg m@mgoldenberg.net

Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
Signed-off-by: Michael Goldenberg <m@mgoldenberg.net>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

❌ Patch coverage is 97.61905% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.93%. Comparing base (777ce05) to head (d528404).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
crates/matrix-sdk-base/src/store/traits.rs 97.61% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6547      +/-   ##
==========================================
+ Coverage   89.92%   89.93%   +0.01%     
==========================================
  Files         381      381              
  Lines      106860   106901      +41     
  Branches   106860   106901      +41     
==========================================
+ Hits        96095    96145      +50     
+ Misses       7108     7099       -9     
  Partials     3657     3657              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 7, 2026

Merging this PR will not alter performance

✅ 50 untouched benchmarks


Comparing mgoldenberg:sync-other-save-locked-state-store-fns (d528404) with main (777ce05)

Open in CodSpeed

@mgoldenberg mgoldenberg marked this pull request as ready for review May 7, 2026 17:14
@mgoldenberg mgoldenberg requested a review from a team as a code owner May 7, 2026 17:14
@mgoldenberg mgoldenberg requested review from andybalaam and removed request for a team May 7, 2026 17:14
@poljar poljar requested review from poljar and removed request for andybalaam May 8, 2026 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ensure all relevant SaveLockedStateStore functions are synchronized

1 participant